-
Notifications
You must be signed in to change notification settings - Fork 2.9k
fix: prevent duplicate tool_result in subtask delegation (EXT-665) #11039
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Check ALL user messages in parentApiMessages for existing tool_result with the same tool_use_id before appending a new one - Previously only checked the last message, missing duplicates in earlier messages - Log warning when skipping duplicate tool_result for debugging - Add tests for duplicate detection and normal case scenarios
Review complete. No issues found. The new commit correctly addresses the EXT-665 root cause by appending the Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
Root cause: In NewTaskTool.execute(), pushToolResult was called AFTER delegateParentAndOpenChild(), but delegateParentAndOpenChild disposes the parent task. This meant the tool_result was lost, and flushPendingToolResultsToHistory generated a placeholder "interrupted" tool_result instead. Fix: Push the tool_result BEFORE calling delegateParentAndOpenChild. Since the child taskId is not yet known, use a generic message. The actual completion result is injected by reopenParentFromDelegation when the child completes. Also keep the defensive duplicate check in reopenParentFromDelegation for backward compatibility with any existing task histories.
|
@roomote I'm not sure this addresses the root cause. Looking at the evidence in EXT-665, there's something weird happening with the timeline: The duplicate tool_result appears at timestamp 1769571245532, but the assistant's tool_use that created that tool_use_id (toolu_01SnH3c7xgVdfLc2md4Fk6yB) wasn't made until timestamp 1769571245577. How is a tool_result appearing BEFORE the tool_use that it's supposedly responding to? This looks like history corruption or old messages being replayed incorrectly during the delegation flow, not just a missing validation check. The defensive fixes here will prevent the duplicate from causing an API error, but they don't explain or fix why we're seeing time-travel tool results in the first place. Can you investigate what's actually causing messages to appear out of chronological order? |
…creating new one True root cause: When reopenParentFromDelegation created a NEW user message with only the new_task tool_result, validateAndFixToolResultIds would see the assistant message expecting BOTH tool_results (e.g., update_todo_list + new_task) but the new message only had new_task, causing a placeholder to be generated for update_todo_list. Fix: Find the existing user message that was created by flushPendingToolResultsToHistory (which already has other tool_results like update_todo_list) and APPEND the new_task tool_result to it instead of creating a new message. This ensures all tool_results for a turn are in the same user message, matching what the Anthropic API expects.
🤖 Roo Code is on it!Working on investigating the timestamp ordering issue in duplicate tool_result detection. ✅ Investigation CompleteRoot Cause AnalysisThe timestamp ordering issue (tool_result at ts 1769571245532 appearing BEFORE tool_use at ts 1769571245577) occurs due to the following sequence:
Fix AppliedThe fix (commit
This prevents the creation of new user messages that would trigger Key Code Changes
|
Problem
EXT-665: When a subtask delegation completes and the parent resumes, the Anthropic API returns a 400 error:
Root Cause Analysis
The issue occurs when an assistant turn contains multiple tool calls (e.g.,
update_todo_list+new_task) and the delegation flow creates the tool_result messages incorrectly.Sequence of Events
update_todo_listANDnew_taskin the same messageupdate_todo_listtool_result (and a placeholder "interrupted" fornew_task)reopenParentFromDelegationreopenParentFromDelegationwas creating a NEW user message with only thenew_tasktool_resultupdate_todo_list+new_tasktool_results, but this new message only hasnew_task, so it generates a placeholder forupdate_todo_listupdate_todo_listtool_results - one in the original message and one placeholder in the new messageWhy Two Tool Results?
The Anthropic API protocol requires that each
tool_useblock in an assistant message must have exactly one correspondingtool_resultblock in the following user message. When multiple tools are called in one turn, all their tool_results must be in the SAME user message.Solution
Primary Fix (ClineProvider.ts)
Instead of creating a new user message, append the
new_tasktool_result to the existing user message that was created byflushPendingToolResultsToHistory. This ensures all tool_results for a multi-tool turn stay together.Supporting Fix (NewTaskTool.ts)
Push the tool_result BEFORE delegation to prevent it from being lost when the parent task is disposed:
Defensive Check (kept for backward compatibility)
The duplicate detection check in
reopenParentFromDelegationis kept as a safety net:Testing
Added comprehensive tests:
reopenParentFromDelegation skips duplicate tool_result when one already exists in history (EXT-665)- Tests the defensive duplicate detectionreopenParentFromDelegation appends to existing user message when multiple tools in one turn (EXT-665 root cause)- Tests the primary fix for multi-tool turnsAll tests pass:
Related